inc/class-geolocation.php (1)

399-424: Remove or use these unused private methods.

Static analysis correctly identifies that supports_geolite2() and is_geolocation_enabled() are never called. Since they're private, they cannot be used externally.

Options:

1. Remove them if not needed
2. Use them where appropriate (e.g., supports_geolite2() could guard the DB lookup in geolocate_via_db())
3. If intended for future use, add a @todo annotation explaining the plan

♻️ Option 1: Remove unused methods

-    /**
-     * Check if server supports MaxMind GeoLite2 Reader.
-     *
-     * `@return` bool
-     */
-    private static function supports_geolite2(): bool
-    {
-        // Check if WooCommerce's MaxMind reader is available
-        if (class_exists('MaxMind\Db\Reader')) {
-            return true;
-        }
-
-        // Check if WooCommerce is installed with the reader
-        if (defined('WC_ABSPATH')) {
-            $reader_path = WC_ABSPATH . 'vendor/maxmind-db/reader/src/MaxMind/Db/Reader.php';
-            return file_exists($reader_path);
-        }
-
-        return false;
-    }
-
-    /**
-     * Check if geolocation is enabled.
-     *
-     * `@param` string $current_settings Current geolocation settings.
-     * `@return` bool
-     */
-    private static function is_geolocation_enabled(string $current_settings): bool
-    {
-        return in_array($current_settings, ['geolocation', 'geolocation_ajax'], true);
-    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@inc/class-geolocation.php` around lines 399 - 424, These two private methods,
supports_geolite2() and is_geolocation_enabled(), are never called; either
remove them or wire them into existing logic: if you don't need them, delete
both methods; otherwise call supports_geolite2() from geolocate_via_db() to
guard MaxMind DB lookups instead of duplicating checks, and replace any ad-hoc
geolocation setting checks with is_geolocation_enabled($current_settings) where
the code currently tests for 'geolocation'/'geolocation_ajax'; if you intend to
keep them for future work, add a `@todo` explaining planned usage.

inc/class-geolocation.php (1)

228-232: ⚠️ Potential issue | 🟠 Major

state remains permanently empty; this regresses existing consumers.

geolocate_ip() hardcodes 'state' => '' and geolocate_via_db() only returns country. Callers in inc/models/class-customer.php and checkout paths expect state to be meaningful, so this now stores/displays empty state data.

Also applies to: 274-291

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@inc/class-geolocation.php` around lines 228 - 232, geolocate_ip() currently
hardcodes 'state' => '' (and geolocate_via_db() only returns country), which
breaks callers expecting a meaningful state; update both functions so they
extract and return a state/region value from the geolocation provider response
(e.g., use the region/region_code/ subdivision field from the MaxMind/DB record
or provider response), fall back to an empty string only if no state/region is
available, and ensure the returned array keys remain 'ip','country','state' so
callers in class-customer and checkout continue to work; update
geolocate_via_db() to include the same 'state' extraction logic (see
geolocate_ip(), geolocate_via_db()) and any local variables like $record,
$country_code to locate where to read the region.

inc/class-geolocation.php (1)

343-356: Remove or expose unused private methods.

Static analysis flags supports_geolite2() and is_geolocation_enabled() as unused. If these are intended for future use or plugin extensibility, consider making them public. Otherwise, remove them to reduce dead code.

♻️ Option 1: Remove if not needed

-	/**
-	 * Check if server supports MaxMind GeoLite2 Reader.
-	 *
-	 * `@return` bool
-	 */
-	private static function supports_geolite2(): bool {
-		// Check if MaxMind reader is available via Composer autoloader
-		return class_exists('MaxMind\Db\Reader');
-	}
-
-	/**
-	 * Check if geolocation is enabled.
-	 *
-	 * `@param` string $current_settings Current geolocation settings.
-	 * `@return` bool
-	 */
-	private static function is_geolocation_enabled(string $current_settings): bool {
-		return in_array($current_settings, ['geolocation', 'geolocation_ajax'], true);
-	}

♻️ Option 2: Make public for extensibility

-	private static function supports_geolite2(): bool {
+	public static function supports_geolite2(): bool {
...
-	private static function is_geolocation_enabled(string $current_settings): bool {
+	public static function is_geolocation_enabled(string $current_settings): bool {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@inc/class-geolocation.php` around lines 343 - 356, The two private static
methods supports_geolite2() and is_geolocation_enabled() are flagged as unused;
either remove both methods to eliminate dead code, or change their visibility to
public static (and update their docblocks) so they are intentionally exposed for
external use/extension; if choosing to expose, ensure no internal call sites
rely on them remaining private and add a unit test or inline comment stating
they are part of the public API for clarity.

tests/WP_Ultimo/Geolocation_Test.php (2)

432-442: Consider verifying cache hit via reflection.

The test confirms the results are equal, but doesn't verify that the second call actually came from the in-memory cache. You could inspect $memory_cache via reflection to confirm it contains the IP after the first call.

💡 Enhanced test suggestion

public function test_geolocate_ip_caching() {
    $ip = '127.0.0.1';

    // First call
    $result1 = Geolocation::geolocate_ip($ip);

    // Verify cache was populated
    $reflection = new \ReflectionClass(Geolocation::class);
    $memory_cache = $reflection->getProperty('memory_cache');
    if (PHP_VERSION_ID < 80100) {
        $memory_cache->setAccessible(true);
    }
    $cache_value = $memory_cache->getValue();
    $this->assertArrayHasKey($ip, $cache_value);

    // Second call should return from cache
    $result2 = Geolocation::geolocate_ip($ip);

    $this->assertEquals($result1, $result2);
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/WP_Ultimo/Geolocation_Test.php` around lines 432 - 442, Update
test_geolocate_ip_caching to assert that the in-memory cache was populated after
the first call: after calling Geolocation::geolocate_ip($ip) use ReflectionClass
on Geolocation to get the memory_cache property (handle accessibility for older
PHP versions by calling setAccessible(true) when needed), read the property
value and assert the IP key exists in that array/structure, then proceed to call
Geolocation::geolocate_ip($ip) again and assert results equal; reference
Geolocation::geolocate_ip and the memory_cache property to locate where to add
the reflection check.

---

234-245: Consider adding a negative test for supports_geolite2() when the class doesn't exist.

The current test only verifies the happy path (Composer package installed). For completeness, you could mock or test the scenario where MaxMind\Db\Reader is not available, though this is difficult without unloading the class.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/WP_Ultimo/Geolocation_Test.php` around lines 234 - 245, Add a negative
test method (e.g., test_supports_geolite2_returns_false_when_class_missing) that
verifies Geolocation::supports_geolite2() returns false when MaxMind\Db\Reader
is unavailable: run it in isolation (use `@runInSeparateProcess` or spawn a
separate PHP process) so the MaxMind class cannot be autoloaded, use
ReflectionClass(Geolocation::class) to get the supports_geolite2 method (as in
the existing test) and invoke it (setAccessible when PHP_VERSION_ID < 80100) and
assertFalse on the result; if your environment supports extensions, you can also
use uopz or runkit to undefine the MaxMind class before calling
supports_geolite2 to simulate the missing-class scenario.